🔒 OAuth 2.1 Penetration Test Report

Target: https://[REDACTED].europe-west1.run.app
Date: 2026-03-19
Scope: OAuth 2.1 authorization flow, DCR, token endpoint, login page, security headers

Summary

🔴 Findings (FAIL)

F1: DCR accepts dangerous redirect_uri schemes (HIGH — upstream)

Component: MCP Python SDK RegistrationHandler (not our code)
Impact: A malicious client can register with javascript:, data:, or file: redirect URIs. If a user authorizes such a client, the auth code could be exfiltrated.

POST /register
{"redirect_uris": ["javascript:alert(document.cookie)"], ...}
→ HTTP 200 (accepted)

Mitigation: This is an upstream issue in the MCP SDK's DCR validation. Our PasswordOAuthProvider inherits the behavior. A workaround would be to override register_client() to reject non-HTTPS schemes. Recommend filing upstream.

F2: No rate limiting on DCR endpoint (MEDIUM — upstream)

20/20 client registrations succeeded without throttling. An attacker could exhaust server memory by registering unlimited clients.

Mitigation: Same as F1 — inherited from InMemoryOAuthProvider. Could override register_client() with a counter/cap.

F3: Redirect URI case-insensitive hostname match (LOW — likely RFC-compliant)

https://PENTEST.EXAMPLE.COM/callback was accepted when https://pentest.example.com/callback was registered. Per RFC 3986 §3.2.2, hostnames are case-insensitive, so this is technically correct. OAuth 2.1 recommends exact string match, but URL normalization makes this equivalent.

Risk: Minimal in practice — attacker still needs to control the domain.

🟡 Findings (WARN)

W1: No clickjacking protection on /login page

Missing X-Frame-Options and CSP frame-ancestors headers. The login form could be embedded in an attacker's iframe to trick users.

Recommendation: Add X-Frame-Options: DENY to login page responses.

W2: No Content-Security-Policy on /login page

The login page HTML is static with no external resources, so the risk is low. Adding a restrictive CSP would defend-in-depth.

Recommendation: Add Content-Security-Policy: default-src 'none'; style-src 'unsafe-inline'; frame-ancestors 'none'

W3-W4: Missing Strict-Transport-Security and X-Content-Type-Options

These should be set by the reverse proxy (Cloud Run handles HSTS for *.run.app domains). Not a code-level issue.

✅ Findings (PASS)

Recommendations

1. Should fix (in this PR): W1+W2 — add security headers to login page HTML responses
2. Should file upstream: F1+F2 — DCR redirect_uri scheme validation + rate limiting in MCP Python SDK
3. No action needed: W3-W4 (infrastructure-level), F3 (RFC-compliant)

Tested with automated OAuth2 penetration testing against the live Cloud Run deployment. Authorization: project owner. Tool: custom Python test harness following OWASP OAuth Testing Guide.

Greptile Summary

This PR adds opt-in OAuth 2.1 authentication (--auth oauth) for remote MCP deployments by subclassing FastMCP's InMemoryOAuthProvider with a password-based login page, brute-force protection, and Dynamic Client Registration — enabling direct integration with claude.ai custom connectors. The implementation is well-structured and all previously raised security concerns (TTL enforcement, HTTPS validation, path-component rejection, assert→ValueError guard, multi-instance documentation, process-list password warning) have been addressed in follow-up commits.

Key observations:

• _render_login skips global lockout check — the GET handler renders the form even when _global_lockout_until is active; the user fills in the form and submits, only to receive a 429 on the POST. The same early-return present in _process_login should also be added to _render_login.
• No security-event logging — lockout triggers and per-request exhaustion are completely silent in the logs, making it impossible for operators to detect brute-force attacks in their Cloud Run / Fly.io log streams.
• Redundant per-request lockout check — _process_login deletes the pending entry at line 157 but re-checks the same condition at line 174 via the still-live local reference; the logic is correct but confusing; consolidating into a single path would improve readability.
• Test coverage is thorough (unit + integration, all lockout paths, TTL on GET and POST, security headers, .well-known metadata).

Confidence Score: 4/5

• Safe to merge — no critical security or correctness bugs; the three flagged items are UX/observability/code-clarity improvements.
• All previously raised security concerns have been resolved. The remaining issues are: (1) a UX inconsistency where the login form renders during an active global lockout, (2) absence of security-event log lines making attack detection impossible in production, and (3) a slightly confusing double-check of the per-request lockout state. None of these affect the correctness of the authentication mechanism or introduce exploitable vulnerabilities.
• linkedin_mcp_server/auth.py — review the three style comments around _render_login global lockout, security logging, and the redundant exhaustion check in _process_login.

Important Files Changed

Sequence Diagram

sequenceDiagram
    participant Client as claude.ai / MCP Client
    participant Server as LinkedIn MCP Server
    participant Login as /login page

    Client->>Server: POST /mcp (no token)
    Server-->>Client: 401 WWW-Authenticate: Bearer

    Client->>Server: GET /.well-known/oauth-authorization-server
    Server-->>Client: 200 { authorization_endpoint, token_endpoint, registration_endpoint }

    Client->>Server: POST /register (DCR)
    Server-->>Client: 200 { client_id, ... }

    Client->>Server: GET /authorize?client_id=...&code_challenge=...&state=...
    Note over Server: authorize() stores pending_auth_request<br/>(request_id, params, created_at, TTL=10min)
    Server-->>Client: 302 → /login?request_id=<token>

    Client->>Login: GET /login?request_id=<token>
    Note over Login: Check TTL & global lockout
    Login-->>Client: 200 Password form

    Client->>Login: POST /login { request_id, password }
    Note over Login: Check TTL, global lockout (60s window),<br/>compare_digest(), per-request counter (max 5)
    alt Correct password
        Login-->>Client: 302 → redirect_uri?code=...&state=...
        Client->>Server: POST /token { code, code_verifier }
        Server-->>Client: 200 { access_token }
        Client->>Server: POST /mcp (Bearer token)
        Server-->>Client: 200 MCP response
    else Wrong password (< 5 attempts)
        Login-->>Client: 200 Form with error + remaining count
    else Wrong password (5th attempt)
        Login-->>Client: 403 Request invalidated
    else 20 global failures in 5 min
        Login-->>Client: 429 Global lockout (60s)
    end

This is a comment left during a code review.
Path: linkedin_mcp_server/auth.py
Line: 113-126

Comment:
**`_render_login` skips global lockout check**

`_render_login` (GET) validates the TTL but does not check `self._global_lockout_until`. During an active global lockout the login form is still rendered and the user can fill it in — only to receive a 429 when they submit (POST). Adding the same early-return here gives the user an immediately actionable message instead of a two-step confusion:

```suggestion
    async def _render_login(self, request: Request) -> Response:
        request_id = request.query_params.get("request_id", "")
        pending = self._pending_auth_requests.get(request_id) if request_id else None
        if not pending:
            return _html_response("Invalid or expired login request.", status_code=400)

        if time.time() - pending["created_at"] > _PENDING_REQUEST_TTL_SECONDS:
            del self._pending_auth_requests[request_id]
            return _html_response(
                "Login request expired. Please restart the authorization flow.",
                status_code=400,
            )

        if time.time() < self._global_lockout_until:
            return _html_response(
                "Too many failed login attempts. Please try again later.",
                status_code=429,
            )

        return _html_response(self._login_html(request_id))
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: linkedin_mcp_server/auth.py
Line: 153-186

Comment:
**No logging of security events**

The rate-limiting and lockout paths are completely silent. Operators deploying to Cloud Run / Fly.io have no visibility into brute-force attempts without log entries. At minimum, the global lockout trigger and per-request exhaustion should emit a `WARNING`:

```python
# When global lockout is triggered (line ~167):
import logging
logger = logging.getLogger(__name__)
logger.warning(
    "Global OAuth lockout triggered: %d failures in %.0fs window",
    len(self._global_failed_attempts),
    _GLOBAL_RATE_LIMIT_WINDOW_SECONDS,
)

# When per-request attempts exhausted (line ~157):
logger.warning(
    "OAuth request %s exhausted per-request attempts (client_id=%s)",
    request_id,
    pending.get("client_id"),
)
```

Without these, a sustained dictionary attack would be entirely invisible in the server logs.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: linkedin_mcp_server/auth.py
Line: 154-178

Comment:
**Redundant per-request lockout check**

The per-request lockout is checked and the entry deleted at lines 156–157, but the same condition is re-checked at line 174. Because `pending` is still a live local reference after `del self._pending_auth_requests[request_id]`, the logic is correct — but the second check (`pending.get("failed_attempts", 0) >= _MAX_FAILED_ATTEMPTS`) is unreachable when the global-lockout path at lines 166–172 triggers first, and is otherwise always true if we reach that point. Consider consolidating to a single return after the deletion:

```python
if not secrets.compare_digest(password, self._password):
    pending["failed_attempts"] = pending.get("failed_attempts", 0) + 1
    per_req_exhausted = pending["failed_attempts"] >= _MAX_FAILED_ATTEMPTS
    if per_req_exhausted:
        del self._pending_auth_requests[request_id]

    self._global_failed_attempts = [
        t for t in self._global_failed_attempts
        if now - t < _GLOBAL_RATE_LIMIT_WINDOW_SECONDS
    ]
    self._global_failed_attempts.append(now)
    if len(self._global_failed_attempts) >= _GLOBAL_MAX_FAILED_ATTEMPTS:
        self._global_lockout_until = now + _GLOBAL_LOCKOUT_SECONDS
        return _html_response(
            "Too many failed login attempts. Please try again later, "
            "then restart the authorization flow from your client.",
            status_code=429,
        )

    if per_req_exhausted:
        return _html_response(
            "Too many failed attempts. Please restart the authorization flow.",
            status_code=403,
        )
    remaining = _MAX_FAILED_ATTEMPTS - pending["failed_attempts"]
    return _html_response(
        self._login_html(request_id, error=f"Invalid password. {remaining} attempt(s) remaining."),
        status_code=200,
    )
```

This removes the ambiguity around reading `pending` after it has been removed from the dict and makes the control flow unambiguous.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: "fix(auth): enforce T..."}

All three issues from review round 2 addressed in 9ef05c6:

1. base_url path-component validation — Added urlparse path check in _validate_oauth(). URLs like https://example.com/api are rejected at startup with a clear error.

2. Misleading 429 on simultaneous lockout collision — 429 message now includes restart guidance: "…try again later, then restart the authorization flow from your client."

3. "Client not found" recovery guidance — Error message now says: "Client registration not found. Please restart the authorization flow from your client."

All with test coverage. CI passing.

Known limitation: OAuth protects the MCP endpoint, but LinkedIn session cookies (used for scraping) are still lost on container restart. Deployments on ephemeral runtimes like Cloud Run need a separate storage backend to persist auth state across cold starts.

Follow-up: feat: GCS-backed auth state persistence for Cloud Run